Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: replace notes with background in generated examples #11516

Closed
wants to merge 2 commits into from

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Jul 12, 2020

Issue: #10796

What I did

In this PR I introduce two things, done together for convenience:
1 - Removal of notes addon, given that it's deprecated in favor of docs.
2 - Addition of backgrounds usage so we can still have a story using parameters.

How to test

Modify scripts/run-e2e.ts under initStorybook:

await exec(`node ../../storybook/lib/cli/dist/generate init`, { cwd });
// await exec(`npx -p @storybook/cli sb init --yes ${type}`, { cwd });

Then run:

yarn test:e2e-framework react   (or other framework of your choice)

From there you can access http://localhost:4000/?path=/story/button--with-custom-background and see it.

  • Is this testable with Jest or Chromatic screenshots?
    Yes

yannbf added 2 commits July 12, 2020 17:10
@yannbf yannbf added the cli label Jul 12, 2020
@yannbf yannbf self-assigned this Jul 12, 2020
@github-actions
Copy link
Contributor

Fails
🚫

PR is not labeled with one of: ["cleanup","doc-dependencies:update","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS against c62c2ba

@shilman shilman added the maintenance User-facing maintenance tasks label Jul 14, 2020
text: 'Defined via addon-backgrounds!',
};

WithCustomBackground.storyName = 'With custom background';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question, will remove it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shilman would you like me to remove the same for ButtonWithLinkToAnotherStory ?

Emoji.parameters = { notes: 'My notes on a button with emojis' };
export const WithCustomBackground = Template.bind({});
WithCustomBackground.args = {
text: 'Defined via addon-backgrounds!',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we just make a story called Inverse that:

  • Inverts the button background
  • Inverts the story background

I think this would fit well into the new template. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love it

@stale
Copy link

stale bot commented Aug 8, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Aug 8, 2020
@ndelangen
Copy link
Member

Is this still being worked on?

@stale stale bot removed the inactive label Aug 17, 2020
@yannbf
Copy link
Member Author

yannbf commented Aug 18, 2020

Is this still being worked on?

Things have changed a lot since then, I don't think this is necessary anymore, as notes have already been removed with the new CLI files (button/page/header) and also backgrounds has presets now. As a result I'll be closing this PR.

Still, it might be good to add an example of parameters usage, like @shilman mentioned earlier like having one of the button stories setting the background and inverting the color. If that is still desired I can open a new PR with that feature!

@yannbf yannbf closed this Aug 18, 2020
@stof stof deleted the fix/remove-addon-notes-usage-in-cli-examples branch May 25, 2022 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants